Skip to content

Conversation

@geruh
Copy link
Contributor

@geruh geruh commented Jan 11, 2026

Rationale for this change

This PR adds table UUID validation on refresh and commit to detect when a table has been replaced. For example, if a table is dropped and recreated with the same name, this prevents accidentally operating on a different table than expected.

Modeled after the Java implementation.

https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java#L202-L209

Python was missing this check.

Are these changes tested?

Added some tests at the table and catalog level

Are there any user-facing changes?

no

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this!

Comment on lines +1506 to +1509
# Only check UUID for existing tables, not new tables
if not isinstance(self, StagedTable):
self._check_uuid(response.metadata)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the scenario where the commit response has a different table uuid?
The commit request should include AssertTableUUID so I would expect the catalog to verify that

"""Return the snapshot references in the table."""
return self.metadata.refs

def _check_uuid(self, new_metadata: TableMetadata) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i find this easier to read as a staticmethod, wdyt?

@staticmethod
def _check_uuid(current_metadata: TableMetadata, new_metadata: TableMetadata)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants